Add QUBODrivers 0.6 benchmark conformance - #27
Conversation
bernalde
left a comment
There was a problem hiding this comment.
Blocking issues: 2
Nonblocking issues: none.
Questions: none.
Tests run and outcomes:
julia --project=. -e 'using Pkg; Pkg.test()'passed with released dependencies:Compatibility metadata6/6,QUBODrivers124/124,Julia C ABI bridge21/21,C ABI contract21/21. The C ABI source syntax check was skipped becauseMQLIB_UPSTREAM_DIRis not set.julia --project=/tmp/mqlib-issue26-conformance -e 'using Test, MQLib; ... QUBODrivers.test(MQLib.Optimizer; examples=false, benchmark_conformance=true) ...'passed against the QUBODrivers #52 checkout:MQLib benchmark conformance115/115, includingBenchmark Conformance14/14.julia --project=/tmp/mqlib-review-qubo-resolve -e 'using Pkg; Pkg.activate(...); Pkg.add(Pkg.PackageSpec(name="QUBO", version="0.6")); Pkg.develop(path=pwd()); Pkg.status()'passed;QUBO v0.6.0resolves with this branch.gh pr checks 27reports all three CI jobs passing.
Merge-readiness: I would not merge this until the blocking issues above are addressed. Since I am the PR author, I am submitting this as COMMENT; the formal approval or request-changes verdict needs to come from another maintainer.
| QUBOTools = "0.12, 0.13" | ||
| QUBODrivers = "0.4, 0.5, 0.6" | ||
| QUBOTools = "0.13" | ||
| QUBODrivers = "0.6" |
There was a problem hiding this comment.
Blocking: this compat still allows the only registered QUBODrivers 0.6 tag, v0.6.0, but that tag predates the conformance API from JuliaQUBO/QUBODrivers.jl#52. Under the PR's normal dependency resolution and CI, validate_metadata, honors_final_reads, and enforces_time_limit are absent, so has_benchmark_metadata_api() is false and the test suite silently runs the legacy QUBODrivers.test path instead of benchmark_conformance = true. That misses issue #26's core acceptance criterion that CI runs the upgraded conformance suite. Please wait for/register a QUBODrivers release containing #52 and constrain compat to that release, or otherwise make CI intentionally test the merged conformance API and fail when it is unavailable.
There was a problem hiding this comment.
Not addressed. I verified there is still no QUBODrivers tag newer than v0.6.0, so there is no released build containing JuliaQUBO/QUBODrivers.jl#52 to constrain compat to. I did not pin CI to an unreleased source or modify workflow files. Leaving this Blocking thread unresolved pending maintainer review.
There was a problem hiding this comment.
Addressed in 47050a8. Compat now requires QUBODrivers = "0.6.1", the trait methods are required at load time, and the test suite always runs QUBODrivers.test(...; benchmark_conformance = true) with no legacy fallback. Verified against released deps: QUBODrivers v0.6.1, Benchmark Conformance 14/14.
| effective_time::Real, | ||
| ) | ||
| if isdefined(QUBODrivers, :_sampler_metadata) | ||
| metadata = QUBODrivers._sampler_metadata( |
There was a problem hiding this comment.
Blocking: with the currently allowed registered QUBODrivers v0.6.0, this helper exists but QUBODrivers does not post-stamp the configured seed into metadata. Because _mqlib_metadata does not pass a seeds dictionary, solving with MQLib.RandomSeed() set emits metadata["seeds"] == Dict() under the dependency version this PR allows. That violates issue #26's requirement to record seeds under metadata["seeds"]. Please pass the configured seed into this helper and initialize seeds = Dict("sampler" => seed) when it is non-nothing, or tighten compat to a QUBODrivers release that reliably stamps the seed and make CI exercise that release.
There was a problem hiding this comment.
Addressed in 1768fc2. _mqlib_metadata now receives the configured RandomSeed and sets metadata["seeds"]["sampler"] for both metadata paths. test_public_c_api_bool_max_objectives now asserts this under released dependencies. Tests: Pkg.test, QUBODrivers #52 conformance, QUBO v0.6 coexistence, native C ABI smoke.
|
Declined Blocking comment needing maintainer decision: Commits pushed:
Main changes:
Tests run:
Comments intentionally not addressed:
Remaining risks or follow-up:
|
|
Update for released QUBODrivers v0.6.1: Commits pushed:
Main changes:
Tests run:
Comments intentionally not addressed:
Remaining risks or follow-up:
|
bernalde
left a comment
There was a problem hiding this comment.
Blocking issues: none.
Nonblocking issues: 1
- CHANGELOG compatibility wording is stale. It says QUBODrivers 0.6, but the PR now requires
QUBODrivers = "0.6.1"; update the entry so release notes do not imply v0.6.0 remains acceptable.
Questions: none.
Tests run and outcomes:
julia --project=. -e 'using Pkg; Pkg.test()': passed with releasedQUBODrivers v0.6.1;Compatibility metadata6/6,QUBODrivers141/141,Julia C ABI bridge30/30,C ABI contract21/21. The C ABI source syntax check was skipped becauseMQLIB_UPSTREAM_DIRis not set.- Focused released-dependency conformance command: passed 115/115, including
Benchmark Conformance14/14;supports_seed=true,honors_final_reads=true,enforces_time_limit=true. - QUBO v0.6 coexistence check in a temporary environment: passed; resolved
QUBO v0.6.0, localMQLib v0.6.0,QUBODrivers v0.6.1, andQUBOTools v0.13.0. bash test/native_c_api_smoke.sh: passed against upstream MQLib5854962.git diff --check: passed.gh pr checks 27: all three CI jobs passed.
Merge-readiness: merge-ready from a correctness standpoint; only the nonblocking changelog wording remains. Since I am the PR author, this review is submitted as COMMENT; formal approval must come from another maintainer.
|
|
||
| ## Unreleased | ||
|
|
||
| - Updated compatibility to QUBODrivers 0.6 and QUBOTools 0.13. |
There was a problem hiding this comment.
Nonblocking: This says QUBODrivers 0.6, but the PR now requires QUBODrivers = "0.6.1". Please update the entry to name 0.6.1 so the release notes do not imply v0.6.0 remains acceptable.
There was a problem hiding this comment.
Addressed in c419e68. The changelog now names QUBODrivers 0.6.1 to match the compat bound. Verified with a compat/changelog assertion, Pkg.test, native C ABI smoke, and git diff --check.
|
Commits pushed:
Main changes:
Tests run and results:
Comments intentionally not addressed:
Remaining risks or follow-up items:
|
Refs #26.
Summary
QUBODrivers = "0.6"andQUBOTools = "0.13".algorithm,backend,reads,seeds,status, andtime.effective.QUBODrivers.FinalNumberOfReads()for emitted samples and declare seed, final-read, and time-limit behavior when the QUBODrivers benchmark trait API is available.QUBODrivers.test(...; benchmark_conformance = true)when the upgraded test API is present.Tests
julia --project=. -e 'using Pkg; Pkg.test()'QUBODrivers v0.6.0,QUBOTools v0.13.0,MQLib_jll v0.1.2+0.Compatibility metadata6/6,QUBODrivers124/124,Julia C ABI bridge21/21,C ABI contract21/21.julia --project=/tmp/mqlib-issue26-conformance -e 'using Pkg; Pkg.activate("/tmp/mqlib-issue26-conformance"; shared=false); Pkg.develop(path="/tmp/QUBODrivers-issue-50"); Pkg.develop(path=pwd()); Pkg.add("Test"); Pkg.test("MQLib")'/tmp/QUBODrivers-issue-50.Compatibility metadata6/6,QUBODrivers141/141,Julia C ABI bridge30/30,C ABI contract21/21.supports_seed=true,honors_final_reads=true,enforces_time_limit=true.Benchmark Conformance14/14; no trait-skipped sets.julia --project=/tmp/mqlib-issue26-qubo-resolve -e 'using Pkg; Pkg.activate("/tmp/mqlib-issue26-qubo-resolve"; shared=false); Pkg.add(Pkg.PackageSpec(name="QUBO", version="0.6")); Pkg.develop(path=pwd()); Pkg.status()'QUBO v0.6.0plus this MQLib branch withQUBODrivers v0.6.0andQUBOTools v0.13.0.Notes
QUBODrivers v0.6.0predates the trait functions. This branch keeps MQLib loadable with releasedv0.6.0and activates the benchmark metadata/trait/conformance path when the merged API is available.MQLIB_UPSTREAM_DIRis set.